Fix mem limit #286
Fix mem limit #286
Conversation
|
Original Post: Steps to reproduce: in incorrectly refers to $memoryLimit as an array with index [0]. The variable is not an array, so the if statement always evaluates to true and the memoryLimit is ALWAYS interpreted to be 128M regardless of the setting in php.ini Corrected code below: Expected Result: Memory limit should be pulled from php.ini from ini Actual Result: always evaluates to true and uses 128M hardcoded setting from code. @seansan This is not a bug ... it's absolutly corret! With this PR See: http://php.net/manual/en/function.isset.php (Example #1) So please check code before PR :) |
| @@ -183,7 +183,7 @@ protected function _getMemoryLimit() | |||
| { | |||
| $memoryLimit = trim(strtoupper(ini_get('memory_limit'))); | |||
|
|
|||
| if (!isSet($memoryLimit[0])){ | |||
| if (!isSet($memoryLimit)){ | |||
There was a problem hiding this comment.
I don't think this is doing what you think it's doing.
$memoryLimit = trim(strtoupper(ini_get('memory_limit'))); will always set the variable. As far as I can tell, empty($memoryLimit) is the proper check in this case.
There was a problem hiding this comment.
Maybe keep it the way it is?
There was a problem hiding this comment.
For me ... I see no need to change this.
There was a problem hiding this comment.
The value of isset() on an empty string is true. This should be if (empty($memoryLimit)) {.
php > $a = '';
php > var_dump(isset($a));
php shell code:1:
bool(true)
|
@sreichel right you are
So inderdaad seems to be correct (or at least checking for that first char)
…On Tue, 13 Jun 2017 at 18:46, sreichel ***@***.***> wrote:
Original Post:
*Steps to reproduce:*
in app/code/core/Mage/Catalog/Model/Product/Image.php, on line 186,
if (!isSet($memoryLimit[0])) {
incorrectly refers to $memoryLimit as an array with index [0]. The
variable is not an array, so the if statement always evaluates to true and
the memoryLimit is ALWAYS interpreted to be 128M regardless of the setting
in php.ini
if (!isSet($memoryLimit[0])) {
$memoryLimit = "128M";
}
Corrected code below:
if (!isSet($memoryLimit)) {
$memoryLimit = "128M";
}
*Expected Result:*
Memory limit should be pulled from php.ini from ini
*Actual Result:*
always evaluates to true and uses 128M hardcoded setting from code.
------------------------------
@seansan <https://github.com/seansan> This is not a bug ... it's
absolutly corret! $memoryLimit[0] gives you the first char of
$memoryLimit *string*. If ini_get('memory_limit') returns an empty string
limit is set to 128M.
With this PR !isSet($memoryLimit) would be false because it is set - it's
just an empty string - and return value would be string '' (length=0)
See: http://php.net/manual/en/function.isset.php (Example #1
<#1>)
So please check code before PR :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#286 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAn0a9DXhYydaHKrly5mE09hNOdNyf_Sks5sDrzmgaJpZM4N4hy5>
.
|
Described here: https://magento.com/tech-resources/bug-tracking/issue/index/id/1540/